NEP-25 #4043
NEP-25 #4043
Conversation
This reverts commit d392f50.
- Document all implemented features and components - Add usage examples for different type definitions - Confirm backward compatibility with NEP-14 - Mark implementation as complete and tested
- Update FromStackItem_ShouldHandleNullFields test to include all 8 fields - Update ToStackItem_ShouldProduceNullFields test to expect 8 fields - Tests now match the actual ExtendedType implementation which uses 8 fields
|
According to the meeting circular check will be changed |
|
@ajara87 could you ad an UT for this case: FAIL: "namedtypes": {
"boo": {
"type": "Array",
"namedtype": "boo"
}
}I think should be solved know |
@shargon @roman-khimov Should work this case? |
No. Top-level namedtype can't reference another namedtype. Inner fields/values/whatever of it can. |
Thank you @roman-khimov. If I understand correctly the following should result: Examples that should fail due to invalid format Example that should fail due to circular reference Example should be successful |
These are valid to me. Not meaningful, like an array of abstract pointers, but not breaking anything either. |
* Extend ValidateExtendedTypes to check circle references * remove unused usings * Add more tests and change enum * Add assert to valid tests
| if (NamedTypes != null) | ||
| { | ||
| ret.Add(new Map(NamedTypes.ToDictionary(p => (PrimitiveType)p.Key, p => (StackItem)p.Value.ToStackItem(referenceCounter)), referenceCounter)); | ||
| } |
There was a problem hiding this comment.
I have a concern: manifest serialization format doesn't depend on hardforks, and we don't change native Management in any way. It means that there's a tiny possibility of a statediff during an update to this version: if someone deploys contract with extended types, then deployment transaction will fail on old nodes and will succeed on updated nodes.
This fact may probably be ignored, but at least we should know about it.
There was a problem hiding this comment.
If they don't update the node, the state difference will come later, isn't it?
There was a problem hiding this comment.
the state difference will come later,
At Faun height, due to other Faun-specific updates, right. But my concern is that with the current implementation state difference may happen even before Faun if someone submits extended manifest to the chain on contract deploy/update.
AnnaShaleva
left a comment
AnnaShaleva
left a comment
There was a problem hiding this comment.
We need either to fix or to agree on possibility of statediff in #4043 (comment).
Other than that this PR LGTM.
|
Closed in favor of #4283 (master) |
Description
Implement NEP-25 neo-project/proposals#160 (review)
Replace of #3272
Type of change
How Has This Been Tested?
Test Configuration:
Checklist: